This repository was archived by the owner on Apr 12, 2024. It is now read-only.
fix(Angular): deprecate angular.merge #16036
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This function has problems with special object types but since it's not used in core, it is not worth implementing fixes for these cases. A general purpose library like lodash (provides `merge`) should be used instead. Closes angular#12653 Closes angular#14941 Closes angular#15180 Closes angular#15992
gkalpak
reviewed
Jun 6, 2017
src/Angular.js
Outdated
| * @deprecated | ||
| * sinceVersion="1.6.5" | ||
| * This function is deprecated, but will not be removed in the 1.x lifecycle. | ||
| * There are a edge cases (see {@link angular.merge#known-issues known issues}) that are not |
There was a problem hiding this comment.
a edge cases known issues --> ???
There was a problem hiding this comment.
Get rid of the a I guess. Did you read over the (see known issues)?
There was a problem hiding this comment.
Oh, right. I missed (see 😃 So, just remove a then.
src/Angular.js
Outdated
| * using [lodash's merge()](https://lodash.com/docs/4.17.4#merge) instead. | ||
| * | ||
| * @knownIssue | ||
| * This is a list of (known) object types that cannot be / are incorrectly handled by this function: |
There was a problem hiding this comment.
The cannot be / are incorrectly doesn't read that well. I would split it up (e.g. cannot be handled or are handled incorrectly) or change it to cannot be correctly handled or something.
petebacondarwin
approved these changes
Jun 12, 2017
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This function has problems with special object types but since it's not used in core,
it is not worth implementing fixes for these cases.
A general purpose library like lodash (provides
merge) should be used instead.Closes #12653
Closes #14941
Closes #15180
Closes #15992
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Deprecation
What is the current behavior? (You can also link to an open issue here)
Unclear API support
What is the new behavior (if this is a feature change)?
Deprecation / docs update
Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
[ ] Tests for the changes have been added (for bug fixes / features)Other information:
Not sure if this should be marked fix or feat
If we deprecate only in 1.7.0, then the known issue section can still go in 1.6.x